New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tree: add missing div-by-zero error for float div #37774
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @mjibson)
pkg/sql/sem/tree/eval_test.go, line 255 at r1 (raw file):
{`1 % 0`, `zero modulus`}, {`1 / 0`, `division by zero`}, {`1::float / 0::float`, `division by zero`},
why does the test go here instead of in testdata/eval/float_decimal_div
? is it because it's an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mjibson and @rafiss)
pkg/sql/sem/tree/eval_test.go, line 255 at r1 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
why does the test go here instead of in
testdata/eval/float_decimal_div
? is it because it's an error?
Yes, as far as I can tell, the datadriven
package that we use for the eval tests here doesn't know how to expect errors yet. That would be a nice feature addition - for now, this other test has a good way of specifying expected errors.
060e03d
to
2f35aa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also had to remove 2 older tests that expected float div-by-0 to return non-errors.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mjibson and @rafiss)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @mjibson)
2f35aa4
to
0828313
Compare
Previously, dividing a float by 0 would return -Inf, instead of returning a divide by 0 error as expected. Release note (bug fix): dividing floats by zero now returns an error, instead of -Inf.
0828313
to
9cdc5cf
Compare
bors r+ TFTRs! |
37774: tree: add missing div-by-zero error for float div r=jordanlewis a=jordanlewis Previously, dividing a float by 0 would return -Inf, instead of returning a divide by 0 error as expected. Release note (bug fix): dividing floats by zero now returns an error, instead of -Inf. Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
Build succeeded |
Previously, dividing a float by 0 would return -Inf, instead of
returning a divide by 0 error as expected.
Release note (bug fix): dividing floats by zero now returns an error,
instead of -Inf.